Skip to content

Conversation

@erikaraujo
Copy link
Contributor

@erikaraujo erikaraujo commented Apr 24, 2025

This pull request introduces a new CountQueryBuilder to the Tempest database library, enabling efficient SQL COUNT queries with fluent condition chaining. It includes the implementation of the builder, integration with existing query builders, and comprehensive unit and integration tests.

New Feature: CountQueryBuilder

  • Added the CountQueryBuilder class to support building SQL COUNT queries with methods for chaining conditions (where, andWhere, orWhere, etc.) and specifying columns. It integrates with ModelDefinition and TableDefinition for resolving table and field details. (src/Tempest/Database/src/Builder/QueryBuilders/CountQueryBuilder.php)
  • Introduced a count() method in the QueryBuilder class to create instances of CountQueryBuilder, allowing seamless integration with existing query workflows. (src/Tempest/Database/src/Builder/QueryBuilders/QueryBuilder.php)

Supporting Query Statement

  • Implemented the CountStatement class, which compiles the SQL for COUNT queries, including support for optional WHERE clauses and column specification. (src/Tempest/Database/src/QueryStatements/CountStatement.php)

TODOs:

  • Add support for GROUP BY and HAVING (As brought up on discord, this makes more sense on the SELECT query builder only)
  • Add support for aliasing the COUNT (There's no point aliasing the count if the only thing returned is an int)
  • Add support for DISTINCT
  • Include COUNT in a ->select

Caveats (?)

The problem with Tempest's approach of defining the "query type" (select/update/delete) before building the rest of the query can make it difficult to reuse the query conditions.

For example - say you need to implement pagination from a filterable query - you'll need to repeat yourself in order to get the results and also a total count:

$selectQuery = query('books')->select();
$countQuery = query('books')->count();

if ($request->search) { 
    $selectQuery = $selectQuery->where('title LIKE ?', "%{$request->search}%");
    $countQuery = $countQuery->where('title LIKE ?', "%{$request->search}%");
}

//...

$count = $countQuery->execut();

$selectQuery = $selectQuery->limit(10)->offset(10);
$results = $selectQuery->all();

Closes #1164

@erikaraujo
Copy link
Contributor Author

@brendt Not sure if the query builder is marked as experimental or not, but IF we can have breaking changes on this count thing, this PR can be merged as is and I can work on the TODOs on follow-up ones.

@innocenzi innocenzi changed the title feat(database): add Count Query Builder and Statement feat(database): add Count query builder and statement Apr 24, 2025
@aidan-casey
Copy link
Member

Nice PR! As I mentioned on the Discord, I think I'd keep this under the select query builder. I'd envision this being something like:

public function count(string $column = '*'): int
{
    // Do the work, accept where statements.
}

I'd add to the query builder a count helper that could be something like:

public function count(string $column = '*'): int
{
    return $this->select()->count($column);
}

Columns previously specified can simply be ignored, I'd think.

That's just IMO.

@brendt
Copy link
Member

brendt commented Apr 25, 2025

Oh, interesting approach, I admit I also thought it would have be part of the SelectQueryBuilder, but having seen this solution, I like it more, I think. That's because our count queries should only do single counts. Ie. they should return an integer, that's it. On the SQL level, you could also use count on select queries, you could even have more than one count per query; but we don't want to support any of that with our querybuilders. If people have more complex cases, they should build the query themselves.

That's why I like the separation, as it clearly indicates that query()->count() is something on its own, apart from normal select queries.

Not sure if the query builder is marked as experimental or not, but IF we can have breaking changes on this count thing, this PR can be merged as is and I can work on the TODOs on follow-up ones.

The whole ORM is experimental, so we're fine 👍

The problem with Tempest's approach of defining the "query type" (select/update/delete) before building the rest of the query can make it difficult to reuse the query conditions.

This is something we can solve with traits in the long run. I prefer to keep the implementations apart for a while though, to make sure we don't over-abstract up front. Especially in an experimental component like the ORM.

brendt
brendt previously requested changes Apr 25, 2025
@erikaraujo
Copy link
Contributor Author

erikaraujo commented Apr 25, 2025

The problem with Tempest's approach of defining the "query type" (select/update/delete) before building the rest of the query can make it difficult to reuse the query conditions.

This is something we can solve with traits in the long run. I prefer to keep the implementations apart for a while though, to make sure we don't over-abstract up front. Especially in an experimental component like the ORM.

I think you misunderstood me, @brendt, because I don't think trait would solve the problem I described 🤔.

What I mentioned is not the fact that public function where is duplicated on all builders - it's that the fact that you must select a "query builder type" upfront by immediately doing ->select() or ->update() or ->count() makes it difficult to reuse where clauses between multiple queries.

I wonder if QueryBuilder could also have where functions - since all of the builders can use them. That way you could do this:

query('books')
    ->where('title = ?', $title)
    ->select()
    ->groupBy(...)
    ->all();

Or this:

query('books')
    ->select()
    ->where('title = ?', $title)
    ->groupBy(...)
    ->all();

But not this (since GROUP BY is "select only"):

query('books')
    ->groupBy(...)
    ->select()
    ->where('title = ?', $title)
    ->all();

That way we could reuse the built query, like:

$query = query('books')
    ->where('title = ?', $title);

$count = $query->count()->execute();
$results = $query->all();

@brendt
Copy link
Member

brendt commented Apr 25, 2025

Gotcha, yeah I misunderstood entirely 😁

It's an interesting idea, but I suggest we discuss it in another issue, it's out of scope for this PR :)

@erikaraujo erikaraujo requested a review from brendt April 25, 2025 10:55
@erikaraujo
Copy link
Contributor Author

Honestly, since we only return an int, why the hell did I add alias support?

Duh.

Let me remove that, it's useless.

@erikaraujo
Copy link
Contributor Author

@brendt Added support for distinct count and removed the useless alias.
This PR is now (really) ready for review.

Here's how to do it:

query('books')
    ->count('title')
    ->distinct()
    ->execute();

If you do the following, an exception will be thrown, since you must specify a column for distinct:

query('books')
    ->count() // or ->count('*')
    ->distinct()
    ->execute();

@brendt brendt merged commit 22dbe07 into tempestphp:main Apr 26, 2025
1 check passed
@brendt
Copy link
Member

brendt commented Apr 26, 2025

Awesome! Are you up for adding the docs for this as well?

@brendt
Copy link
Member

brendt commented Apr 26, 2025

See #1179 for documentation followup

@erikaraujo
Copy link
Contributor Author

Awesome! Are you up for adding the docs for this as well?

Sure thing, but I may not be able to get around to it until after may 1st.

@erikaraujo erikaraujo deleted the feat/db-count branch April 27, 2025 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get count of records from the DB

3 participants